Guard standalone termination against zero job id by shreyaskommuri · Pull Request #931 · NVIDIA/cloudai

shreyaskommuri · 2026-06-16T05:09:32Z

Summary

Guard standalone job termination so job.id == 0 does not shell out to kill -9 0.
Log a warning and skip termination when the standalone job ID is the dry-run sentinel rather than a launched process.
Add regression coverage for integer and string zero job IDs.

Context

Fixes #929.

While using CloudAI as the execution layer for CloudAI Autotune, a wrapper smoke test hit the common standalone sleep dry-run path. CloudAI scheduled dependency termination for job 0, which produced kill -9 0. On POSIX shells that targets the current process group, so the parent wrapper was killed and exited with code 137 before it could report a normal result.

In standalone dry-run mode, StandaloneRunner._submit_test() uses job_id = 0 because no subprocess is launched. That sentinel should not be passed to the shell as a real kill target.

Test Plan

python -m pytest tests/systems/standalone/test_system.py -q
- 5 passed
python -m pytest -q
- 1574 passed, 4 skipped, 463 deselected
uv run pre-commit run --all-files
- all hooks passed

I also attempted the live standalone sleep dry-run on a fresh origin/main branch, but that path is currently blocked earlier by the already-filed abstract-method issue #920 / PR #921. This PR keeps the scope to the zero-job-id termination fix from #929.

coderabbitai · 2026-06-16T05:09:40Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: c5ab9887-e465-41a9-a088-c2d302862e25

📥 Commits

Reviewing files that changed from the base of the PR and between 030ac94 and b93656e.

📒 Files selected for processing (2)

src/cloudai/systems/standalone/standalone_system.py
tests/systems/standalone/test_system.py

📝 Walkthrough

Walkthrough

StandaloneSystem.kill now validates trimmed job IDs as positive integers before running kill -9. Tests cover invalid PID rejection and normalization of leading-zero numeric strings. The copyright header is updated through 2026.

Changes

PID Validation in Kill Command

Layer / File(s)	Summary
PID parsing and validation in `StandaloneSystem.kill` `src/cloudai/systems/standalone/standalone_system.py`	Parses and validates `job.id`, skips invalid or non-positive PIDs with a warning, and executes `kill -9` using the normalized integer PID.
Invalid PID rejection and normalization tests `tests/systems/standalone/test_system.py`	Verifies invalid IDs do not execute commands and leading-zero numeric IDs are normalized before execution.

Estimated code review effort: 2 (Simple) | ~12 minutes

Suggested reviewers: jeffnvidia

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change: guarding standalone termination when the job ID is zero.
Description check	✅ Passed	The description is directly related to the changeset and accurately explains the zero job ID fix and its regression tests.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

_{Comment @coderabbitai help to get the list of available commands.}

shreyaskommuri · 2026-06-16T05:12:29Z

@coderabbitai review

coderabbitai · 2026-06-16T05:12:35Z

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/cloudai/systems/standalone/standalone_system.py`:
- Around line 81-90: The job ID validation using string-equality check `== "0"`
only blocks the exact string and allows variants like "00", "+0", and "-0" to
pass through, which then parse as integer 0 and signal the entire process group
instead of a specific process. Replace the string comparison with explicit
integer parsing and validation: convert job.id to an integer, validate it is a
positive number (greater than 0), and only proceed with the kill command if that
validation passes. This ensures that all zero variants and invalid PID values
are rejected before constructing the cmd string and calling cmd_shell.execute.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: f5b3c7a8-35f8-4530-bdda-63d1d3531de9

📥 Commits

Reviewing files that changed from the base of the PR and between 1b0e8cc and 7130435.

📒 Files selected for processing (2)

src/cloudai/systems/standalone/standalone_system.py
tests/systems/standalone/test_system.py

shreyaskommuri · 2026-06-16T06:29:53Z

Addressed CodeRabbit feedback by parsing standalone job IDs as integers before constructing the kill command. The guard now skips non-numeric IDs and all non-positive PID variants such as 00, +0, -0, and -1, and uses the normalized positive PID for valid kills. Added regression coverage for those cases plus numeric normalization. Re-ran python -m pytest tests/systems/standalone/test_system.py -q, full python -m pytest -q, and uv run pre-commit run --all-files successfully.

Signed-off-by: shreyaskommuri <shreyaskommuri@gmail.com>

shreyaskommuri · 2026-07-26T21:49:03Z

Rebased onto latest main (was 186 commits behind) to keep this mergeable. No conflicts. Re-ran python -m pytest -q (1764 passed, 18 skipped), ruff check, and ruff format --check on the changed files — all clean. Re-verified the fix against the bundled sleep scenario: dry-run now exits 0 with all jobs PASSED where it previously died with SIGKILL from kill -9 0.

Copilot

Pull request overview

This PR prevents a dangerous standalone termination edge case where a dry-run sentinel job ID (0) could be passed to kill -9 0, potentially killing the caller’s process group. It adds PID validation/normalization before shelling out and includes regression tests to ensure invalid/sentinel IDs are skipped.

Changes:

Validate and normalize StandaloneSystem.kill() targets by parsing job.id to an integer PID and skipping non-positive / non-numeric values with warnings.
Prevent kill -9 0 (and other unsafe/non-PID values like negative IDs) from being executed.
Add test coverage for skipping invalid IDs and normalizing numeric string PIDs (e.g., "0012345" → 12345).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
`src/cloudai/systems/standalone/standalone_system.py`	Adds PID parsing/validation guards in `kill()` to prevent unsafe `kill` invocations for sentinel/invalid job IDs.
`tests/systems/standalone/test_system.py`	Adds regression tests ensuring invalid/sentinel IDs do not invoke `kill` and numeric string PIDs are normalized.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

shreyaskommuri force-pushed the issue/standalone-kill-zero branch from 68ca9fb to 7130435 Compare June 16, 2026 05:12

shreyaskommuri marked this pull request as ready for review June 16, 2026 05:12

shreyaskommuri requested review from jeffnvidia, podkidyshev and srivatsankrishnan as code owners June 16, 2026 05:12

coderabbitai Bot reviewed Jun 16, 2026

View reviewed changes

Comment thread src/cloudai/systems/standalone/standalone_system.py Outdated

shreyaskommuri force-pushed the issue/standalone-kill-zero branch from 7130435 to 030ac94 Compare June 16, 2026 06:29

Guard standalone termination against zero job id

b93656e

Signed-off-by: shreyaskommuri <shreyaskommuri@gmail.com>

Copilot AI review requested due to automatic review settings July 26, 2026 21:48

shreyaskommuri force-pushed the issue/standalone-kill-zero branch from 030ac94 to b93656e Compare July 26, 2026 21:48

Copilot started reviewing on behalf of shreyaskommuri July 26, 2026 21:49 View session

Copilot AI reviewed Jul 26, 2026

View reviewed changes

podkidyshev approved these changes Jul 28, 2026

View reviewed changes

podkidyshev merged commit 32884e0 into NVIDIA:main Jul 28, 2026
5 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Guard standalone termination against zero job id - #931

Guard standalone termination against zero job id#931
podkidyshev merged 1 commit into
NVIDIA:mainfrom
shreyaskommuri:issue/standalone-kill-zero

shreyaskommuri commented Jun 16, 2026

Uh oh!

coderabbitai Bot commented Jun 16, 2026 •

edited

Loading

Walkthrough

Changes

Uh oh!

shreyaskommuri commented Jun 16, 2026

Uh oh!

coderabbitai Bot commented Jun 16, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

shreyaskommuri commented Jun 16, 2026

Uh oh!

shreyaskommuri commented Jul 26, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

shreyaskommuri commented Jun 16, 2026

Summary

Context

Test Plan

Uh oh!

coderabbitai Bot commented Jun 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

shreyaskommuri commented Jun 16, 2026

Uh oh!

coderabbitai Bot commented Jun 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

shreyaskommuri commented Jun 16, 2026

Uh oh!

shreyaskommuri commented Jul 26, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

coderabbitai Bot commented Jun 16, 2026 •

edited

Loading

coderabbitai Bot commented Jun 16, 2026 •

edited

Loading